New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(samples): Talent API Sample Revision & Test #1546
docs(samples): Talent API Sample Revision & Test #1546
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 🎉
👋 @JCIV-SE it looks like the sample tests are failing. Can you take a look please? |
@JustinBeckwith Carrying over the outdated duplicate PR's discussion:
I'm mirroring a lot of the code from other samples. The biggest difference is the url passed to .post() in the test where I hard-code the project-id to the one used for testing in the repo. Using a template literal + project-id variable doesn't work well there. The company creation method constructs the url to access the company directly using the parent property and the request body properties - do you think it has to do with that? I'm new to Nock so the nuances aren't readily apparent yet :) |
🤷♂️ If you can push to a branch on upstream, I can clone it and take a look :) |
@JustinBeckwith Don't have access to the upstream repo like that :l you can clone the forked branch here: https://github.com/JCIV-SE/google-api-nodejs-client/tree/samples/talentapi |
@JCIV-SE I just invited you to the googleapis org. Let me know when you accept, and I can get you added to the correct groups. |
test/samples/test.samples.jobs.ts
Outdated
|
||
it('should create a company', async () => { | ||
const scope = nock(Utils.baseUrl) | ||
.post(`/v3/jobs/project/project-id/companies`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! I think this is the problem. For this url, can you use ${process.env.GCLOUD_PROJECT}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! I tried it several different ways though and I'm still getting:
FetchError: request to https://jobs.googleapis.com/v3/project/project-id/companies failed,
reason: Nock: Disallowed net connect for "jobs.googleapis.com:443/v3/project/project-id/companies"
🎊
I accepted the invitation (thanks!), I'll try pushing a branch upstream once auth allows it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh weird. Share your updated code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Here's version 1:
it('should create a company', async () => {
const scope = nock(Utils.baseUrl)
.post(`/v3/jobs/project/${process.env.GCLOUD_PROJECT}/companies`)
version 2:
it('should create a company', async () => {
const scope = nock(Utils.baseUrl)
.post(`${process.env.GCLOUD_PROJECT}`)
I tried a few other ways but the above two are the ones that make sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, v1 looks right to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some logging revealed that GCLOUD_PROJECT
is undefined
until we go into runSample's scope where we call on the auth service to generate a project Id - copying the same code ( const projectId = await google.auth.getProjectId();
) into the test's scope gives us undefined
too so.. any chance we can erase this test and push the sample up anyway? A bit anarchistic but I noticed there are a few other APIs without them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep the test, just put a .skip
on it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Done!
…ejs-client into samples/talentapi
because the tools are already installed in the docker image as of googleapis/testing-infra-docker#227 Source-Link: googleapis/synthtool@ab7384e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:bb493bf01d28519e82ab61c490c20122c85a7119c03a978ad0c34b4239fbad15
* fix: add hashes to requirements.txt and update Docker images so they require hashes. * fix: add hashes to docker/owlbot/java/src * Squashed commit of the following: commit ab7384ea1c30df8ec2e175566ef2508e6c3a2acb Author: Jeffrey Rennie <rennie@google.com> Date: Tue Aug 23 11:38:48 2022 -0700 fix: remove pip install statements (#1546) because the tools are already installed in the docker image as of googleapis/testing-infra-docker#227 commit 302667c9ab7210da42cc337e8f39fe1ea99049ef Author: WhiteSource Renovate <bot@renovateapp.com> Date: Tue Aug 23 19:50:28 2022 +0200 chore(deps): update dependency setuptools to v65.2.0 (#1541) Co-authored-by: Anthonios Partheniou <partheniou@google.com> commit 6e9054fd91d1b500cae58ff72ee9aeb626077756 Author: WhiteSource Renovate <bot@renovateapp.com> Date: Tue Aug 23 19:42:51 2022 +0200 chore(deps): update dependency nbconvert to v7 (#1543) Co-authored-by: Anthonios Partheniou <partheniou@google.com> commit d229a1258999f599a90a9b674a1c5541e00db588 Author: Alexander Fenster <fenster@google.com> Date: Mon Aug 22 15:04:53 2022 -0700 fix: update google-gax and remove obsolete deps (#1545) commit 13ce62621e70059b2f5e3a7bade735f91c53339c Author: Jeffrey Rennie <rennie@google.com> Date: Mon Aug 22 11:08:21 2022 -0700 chore: remove release config and script (#1540) We don't release to pypi anymore. * chore: rollback java changes to move forward with other languages until Java's docker image is fixed Source-Link: googleapis/synthtool@4826337 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:7fefeb9e517db2dd8c8202d9239ff6788d6852bc92dd3aac57a46059679ac9de
* fix: add hashes to requirements.txt and update Docker images so they require hashes. * fix: add hashes to docker/owlbot/java/src * Squashed commit of the following: commit ab7384ea1c30df8ec2e175566ef2508e6c3a2acb Author: Jeffrey Rennie <rennie@google.com> Date: Tue Aug 23 11:38:48 2022 -0700 fix: remove pip install statements (#1546) because the tools are already installed in the docker image as of googleapis/testing-infra-docker#227 commit 302667c9ab7210da42cc337e8f39fe1ea99049ef Author: WhiteSource Renovate <bot@renovateapp.com> Date: Tue Aug 23 19:50:28 2022 +0200 chore(deps): update dependency setuptools to v65.2.0 (#1541) Co-authored-by: Anthonios Partheniou <partheniou@google.com> commit 6e9054fd91d1b500cae58ff72ee9aeb626077756 Author: WhiteSource Renovate <bot@renovateapp.com> Date: Tue Aug 23 19:42:51 2022 +0200 chore(deps): update dependency nbconvert to v7 (#1543) Co-authored-by: Anthonios Partheniou <partheniou@google.com> commit d229a1258999f599a90a9b674a1c5541e00db588 Author: Alexander Fenster <fenster@google.com> Date: Mon Aug 22 15:04:53 2022 -0700 fix: update google-gax and remove obsolete deps (#1545) commit 13ce62621e70059b2f5e3a7bade735f91c53339c Author: Jeffrey Rennie <rennie@google.com> Date: Mon Aug 22 11:08:21 2022 -0700 chore: remove release config and script (#1540) We don't release to pypi anymore. * chore: rollback java changes to move forward with other languages until Java's docker image is fixed Source-Link: googleapis/synthtool@4826337 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:7fefeb9e517db2dd8c8202d9239ff6788d6852bc92dd3aac57a46059679ac9de Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Fixes #1296